sctp restore #476
Draft
sctp restore #476
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
shinyoshiaki
commented
May 31, 2025
shinyoshiaki
commented
- Refactor SCTP constants and improve data handling in RTCDtlsTransport and InboundStream
- Add SCTPTimerManager for improved timer management in SCTP
- Refactor SCTPTimerManager to use constants for max retransmissions and simplify timer handling
- wip Implement SCTPTransmitter for improved chunk sending and connection state management
- Enhance SCTP connection management by integrating EventDisposer and updating connection state handling in SCTPTransmitter
- Refactor SCTP and SCTPTransmitter for improved state management and timer handling
- Fix SCTPTransmitter event handling by specifying state type in onStateChanged and passing state to execute method
- Refactor SCTPTransmitter to delegate RTT calculations to SCTPTimerManager and remove redundant RTO update logic
- Refactor SCTPTransmitter and SCTPTimerManager to use event subscriptions for timer expirations and improve error handling during data transmission
- Implement SctpReconfig class for managing reconfiguration requests and streamline SCTP state handling
- wip: Enhance SctpReconfig for improved reconfiguration handling and integrate with SCTP class
- wip Refactor SCTP to delegate reconfiguration response handling to SctpReconfig class for improved code organization and maintainability
- wip: Refactor SCTP to delegate stream addition handling to SctpReconfig for improved code organization and maintainability
- refactor: Simplify stream handling in SctpReconfig and update SCTP to manage inbound streams more effectively
- refactor: Update SctpReconfig and SCTP to use localTsn from SCTPTransmitter for improved consistency and clarity
- refactor: Remove unused SCTP RTO constants from transmitter for cleaner code
- refactor: Update SCTP class to improve property access and organization in tests
- refactor: Update SCTP and UdpTransport classes for improved clarity and consistency; add integration test for ping-pong functionality
- refactor: Enhance StreamManager and SCTP classes for improved stream handling and organization; update tests accordingly
- refactor: Improve Chunk and SCTPTimerManager classes with enhanced error handling and parsing methods; add DTO conversion methods
- ** Enhance SCTP, SctpReconfig, StreamManager, and SCTPTimerManager classes with DTO methods for improved data handling and organization**
… and InboundStream
…d simplify timer handling
…on state management
…pdating connection state handling in SCTPTransmitter
…eChanged and passing state to execute method
…ager and remove redundant RTO update logic
…ons for timer expirations and improve error handling during data transmission
…d streamline SCTP state handling
…ntegrate with SCTP class
…tpReconfig class for improved code organization and maintainability
…ig for improved code organization and maintainability
… manage inbound streams more effectively
…mitter for improved consistency and clarity
…nd consistency; add integration test for ping-pong functionality
…handling and organization; update tests accordingly
…ror handling and parsing methods; add DTO conversion methods
…ses with DTO methods for improved data handling and organization
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the SCTP implementation, improving state management, timer handling, reconfiguration logic, and adding session migration support.
- Refactored SCTP, SCTPTransmitter, and related timer and stream management classes to improve error handling and code organization.
- Added session state export/restore functionality for migration and enhanced integration tests, examples, and documentation.
- Updated transport implementations, constants, and dependency configurations for improved maintainability and consistency.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webrtc/src/transport/sctp.ts | Adjusted event subscriptions and error messages for clearer state handling. |
| packages/webrtc/src/transport/dtls.ts | Integrated reactive data handling via onData events. |
| packages/webrtc/src/media/rtpTransceiver.ts | Reordered properties and consolidated codec getters/setters. |
| packages/sctp/tests/utils.ts | Introduced a mock transport for testing. |
| packages/sctp/tests/stream.test.ts | Updated imports to reflect refactored module structure. |
| packages/sctp/tests/session-migration.test.ts | Added tests for session state export and migration. |
| packages/sctp/tests/sctp.test.ts | Modified stream count handling and reconfiguration requests. |
| packages/sctp/tests/integrate.test.ts | Added an integration test for ping-pong functionality. |
| packages/sctp/src/util.ts | Introduced utility functions for TSN arithmetic. |
| packages/sctp/src/transport.ts | Renamed variables for clarity and consistency in UDP transport. |
| packages/sctp/src/timer.ts | Overhauled timer management with improved error handling and DTO conversion. |
| packages/sctp/src/stream.ts | Enhanced stream management, reassembly and DTO conversion methods. |
| packages/sctp/src/reconfig.ts | Improved reconfiguration handling with clearer logging and error responses. |
| packages/sctp/src/index.ts | Updated exports to include new types and timer manager. |
| packages/sctp/src/const.ts | Added new constants and restructured protocol definitions. |
| packages/sctp/src/chunk.ts | Revised chunk parsing and encoding with improved error messages. |
| packages/sctp/package.json | Updated dependencies and added new type definitions. |
| packages/sctp/examples/session-migration.ts | Added an example demonstrating session migration. |
| packages/sctp/README.md | Extended documentation with a session migration guide and usage examples. |
| .vscode/settings.json | Updated settings to include additional snippet keywords. |
| restartT3() { | ||
| this.cancelT3(); | ||
| // for performance | ||
| this.t3Handle = setTimeout(() => this.t3Expired(), this.rto); |
There was a problem hiding this comment.
In restartT3, the timeout value is set using this.rto without multiplying by 1000, which is inconsistent with how timers are started elsewhere (e.g., startT3 uses this.rto * 1000). Consider using 'this.rto * 1000' to ensure consistent timer intervals.
Suggested change
| this.t3Handle = setTimeout(() => this.t3Expired(), this.rto); | |
| this.t3Handle = setTimeout(() => this.t3Expired(), this.rto * 1000); |
| let body = Buffer.from(""); | ||
| let padding = Buffer.from(""); | ||
| params.forEach(([type, value]) => { | ||
| for (const [type, value] of params) { |
There was a problem hiding this comment.
The encodeParams function does not aggregate the encoded parameter segments into the 'body' buffer before returning it. To fix this, concatenate 'paramHeader', 'value', and 'padding' into 'body' inside the loop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.